feat: add rdmAsLun parameter to Plan resource by krcmarik · Pull Request #2754 · RedHatQE/openshift-python-wrapper

krcmarik · 2026-07-02T09:59:58Z

Add rdm_as_lun parameter to the Plan class to support mapping RDM (Raw Device Mapping) disks as LUN devices with SCSI bus on the target VM during MTV migrations. Only applies to vSphere source providers.

Summary by CodeRabbit

New Features
- Added support for an optional rdm_as_lun setting when creating a plan.
- When enabled, this value is included in the generated plan configuration.
- Documented that this option applies to vSphere-based source providers only.

Add rdm_as_lun parameter to the Plan class to support mapping RDM (Raw Device Mapping) disks as LUN devices with SCSI bus on the target VM during MTV migrations. Only applies to vSphere source providers.

coderabbitai · 2026-07-02T10:00:19Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5397de51-e3f8-40b6-9fbc-a5fb5ed8bcc9

📥 Commits

Reviewing files that changed from the base of the PR and between 3d29c21 and 343f639.

📒 Files selected for processing (1)

ocp_resources/plan.py

Walkthrough

Adds an optional rdm_as_lun boolean parameter to the Plan resource class in ocp_resources/plan.py, applicable only to vSphere source providers, storing it on the instance and including it in the generated spec dictionary when set.

Changes

Plan rdm_as_lun support

Layer / File(s)	Summary
rdm_as_lun parameter and spec generation `ocp_resources/plan.py`	Documents `rdm_as_lun` in the class docstring, adds it as an optional constructor argument, stores it as an instance attribute, and conditionally emits `spec["rdmAsLun"]` in `to_dict()` when set.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Related Issues: None found in provided context.

Suggested labels: enhancement

Suggested reviewers: None specified.

🐰 A whisper of LUN, a plan takes shape,
vSphere's disks now find their escape,
One boolean flag, tucked in the spec,
A tiny hop, no need to fret. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is only one sentence and omits the required template sections like details, issue links, special notes, and bug info.	Expand the description to fill the template sections: Short description, More details, Why needed, issue links, Special notes, and Bug.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the main change: adding rdmAsLun support to Plan.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

qodo-code-review · 2026-07-02T10:00:45Z

PR Summary by Qodo

Add rdmAsLun option to Plan resource for vSphere RDM migrations

✨ Enhancement 🕐 10-20 Minutes

AI Description

• Add rdm_as_lun parameter to the Plan resource API.
• Serialize the option into the Plan spec as rdmAsLun when provided.
• Document behavior: map vSphere RDM disks as SCSI LUNs on the target VM.

Diagram

graph TD
  A["Caller code"] --> B["Plan class"] --> C["Plan.to_dict()"] --> D["Plan CR spec"] --> E["MTV/Forklift controller"]

High-Level Assessment

The PR’s approach (add an explicit optional parameter and map it to the CRD field name during serialization) is the most direct and consistent with how other Plan fields are modeled, while keeping API usage discoverable via type hints and docstrings.

Files changed (1) +9 / -0

Enhancement (1) +9 / -0

plan.pyAdd rdm_as_lun Plan option and serialize as rdmAsLun +9/-0

Add rdm_as_lun Plan option and serialize as rdmAsLun

• Introduces a new optional 'rdm_as_lun' constructor parameter on 'Plan' and stores it on the instance. Updates 'to_dict()' to include 'spec.rdmAsLun' when set, and extends the class docstring to document vSphere-only RDM-as-LUN behavior.

ocp_resources/plan.py

redhat-qe-bot1 · 2026-07-02T10:02:28Z

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
Issue Creation: Disabled for this repository
Branch Labeling: Branch-specific labels are applied to track the target branch
Auto-verification: Auto-verified users have their PRs automatically marked as verified
Labels: All label categories are enabled (default configuration)

📋 Available Commands

PR Status Management

/wip - Mark PR as work in progress (adds WIP: prefix to title)
/wip cancel - Remove work in progress status
/hold - Block PR merging (approvers only)
/hold cancel - Unblock PR merging
/verified - Mark PR as verified
/verified cancel - Remove verification status
/reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
/regenerate-welcome - Regenerate this welcome message
/security-override - Set security check runs to pass (maintainers only)
/security-override cancel - Re-run security checks

Review & Approval

/lgtm - Approve changes (looks good to me)
/approve - Approve PR (approvers only)
/automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
/assign-reviewers - Assign reviewers based on OWNERS file
/assign-reviewer @username - Assign specific reviewer
/check-can-merge - Check if PR meets merge requirements

Testing & Validation

/retest tox - Run Python test suite with tox
/retest python-module-install - Test Python package installation
/retest conventional-title - Validate commit message format
/retest all - Run all available tests

Cherry-pick Operations

/cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
- Multiple branches: /cherry-pick branch1 branch2 branch3
/cherry-pick-retry <branch> - Retry a failed cherry-pick (merged PRs only)

Branch Management

/rebase - Rebase this PR branch onto its base branch

Label Management

/<label-name> - Add a label to the PR
/<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

Approval: /approve from at least one approver
Status Checks: All required status checks must pass
No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
Verified: PR must be marked as verified

📊 Review Process

Approvers and Reviewers

Approvers:

myakove
rnetser

Reviewers:

myakove
rnetser

Available Labels

hold
verified
wip
lgtm
approve
automerge

AI Features

Conventional Title: Mode: fix (claude/claude-opus-4-6-1m)
Cherry-Pick Conflict Resolution: Enabled (claude/claude-opus-4-6-1m)

Security Checks

Suspicious Path Detection: Monitors paths: .claude/, .vscode/, .cursor/, .devcontainer/, .pi/, .github/workflows/, .github/actions/
Committer Identity Check: Verifies last committer matches PR author
Mandatory: Security checks block merge (use /security-override to bypass — maintainers only)

💡 Tips

WIP Status: Use /wip when your PR is not ready for review
Verification: The verified label is removed on new commits unless the push is detected as a clean rebase
Cherry-picking: Cherry-pick labels are processed when the PR is merged
Permission Levels: Some commands require approver permissions
Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

qodo-code-review · 2026-07-02T10:03:18Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. vSphere-only flag not enforced 🐞 Bug ⚙ Maintainability

Description

Plan documents rdm_as_lun as “Only applies to vSphere source providers”, but to_dict() will
serialize spec["rdmAsLun"] whenever the parameter is set, regardless of source provider type. This
lets callers generate Plan specs that contradict the class’ documented constraint, making the API
contract ambiguous.

Code

ocp_resources/plan.py[R223-224]

+            if self.rdm_as_lun is not None:
+                spec["rdmAsLun"] = self.rdm_as_lun

Relevance

⭐ Low
Team often rejects adding spec validation/constraints in to_dict (e.g., required/non-empty checks
rejected in PRs #2529, #2520).
PR-#2529
PR-#2520
PR-#2545

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The docstring explicitly states the option is vSphere-only, but the serializer adds rdmAsLun with
no conditional logic tied to source provider type.

ocp_resources/plan.py[45-48]
ocp_resources/plan.py[223-224]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Plan` claims `rdm_as_lun` only applies to vSphere sources, but the implementation always emits `spec.rdmAsLun` when set. This creates an ambiguous contract: the class documentation implies a constraint that the code does not enforce.

### Issue Context
The class currently does not track the source provider type (only the provider name/namespace), so it cannot enforce the constraint unless additional context is supplied.

### Fix Focus Areas
- ocp_resources/plan.py[45-48]
- ocp_resources/plan.py[223-224]

### Suggested fix approaches (pick one)
1) **Enforce the constraint**: add an optional `source_provider_type` (or similar) parameter to `Plan.__init__`, and if `rdm_as_lun is not None` and `source_provider_type != ProviderType.VSPHERE`, raise `ValueError`.
2) **Clarify documentation**: if enforcement is intentionally out-of-scope for this class, adjust the docstring wording to indicate that the field is *only meaningful/used by MTV* for vSphere sources, but will still be serialized if provided.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

feat: add rdmAsLun parameter to Plan resource

343f639

Add rdm_as_lun parameter to the Plan class to support mapping RDM (Raw Device Mapping) disks as LUN devices with SCSI bus on the target VM during MTV migrations. Only applies to vSphere source providers.

redhat-qe-bot1 requested a review from myakove July 2, 2026 10:02

redhat-qe-bot1 assigned krcmarik Jul 2, 2026

redhat-qe-bot1 requested a review from rnetser July 2, 2026 10:02

redhat-qe-bot1 added branch-main size/XS labels Jul 2, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: add rdmAsLun parameter to Plan resource#2754

feat: add rdmAsLun parameter to Plan resource#2754
krcmarik wants to merge 1 commit into
RedHatQE:mainfrom
krcmarik:rdmAsLun

krcmarik commented Jul 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 2, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

qodo-code-review Bot commented Jul 2, 2026

Uh oh!

redhat-qe-bot1 commented Jul 2, 2026

Uh oh!

qodo-code-review Bot commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

krcmarik commented Jul 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

qodo-code-review Bot commented Jul 2, 2026

PR Summary by Qodo

Uh oh!

redhat-qe-bot1 commented Jul 2, 2026

Welcome! 🎉

🔄 Automatic Actions

📋 Available Commands

PR Status Management

Review & Approval

Testing & Validation

Cherry-pick Operations

Branch Management

Label Management

✅ Merge Requirements

📊 Review Process

💡 Tips

Uh oh!

qodo-code-review Bot commented Jul 2, 2026

Code Review by Qodo

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

krcmarik commented Jul 2, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 2, 2026 •

edited

Loading